Fix Tizen linking flags used during ILCompiler build#129844
Conversation
dotnet-policy-service
Bot
added
the
community-contribution
|
Tagging subscribers to this area: @agocke, @dotnet/ilc-contrib |
|
Looks good. Is it for cross-publish only or does it work on Tizen machine as well (assuming SDK works on Tizen)? |
Currently I'm checking cross build case. |
|
|
||
| <!-- Tizen requires additional linker args --> | ||
| <LinkerArg Include="-B"$(_TizenToolchainPath)"" Condition="'$(_IsTizenExitCode)' == '0'" /> | ||
| <LinkerArg Include="-L"$(SysRoot)/$(_TizenToolchainLibDir)"" Condition="'$(_IsTizenExitCode)' == '0'" /> |
There was a problem hiding this comment.
Are these things really necessary? We have not been passing standard library paths in other platforms. We pass --sysroot which should set these things up.
There was a problem hiding this comment.
Some of these maybe necessary, since Tizen might not be using regular/defacto locations in some cases (clang defines them at: https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Gnu.cpp). @gbalykov, could you please try removing a line, then publish an app on armv7l, arm64 and x64 to see if that line was needed; rinse and repeat to minimize this patch? Typically we only need a few linker related args not everything which was needed to build runtime repo.
There was a problem hiding this comment.
I've minimized the patch keeping only what's required. Checked this on runtime build, no issues. Sysroot is passed even without this change, but that's not enough.
Without this change linker flags set in src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets in SetupOSSpecificProps do not consider tizen platform specifics, which are set in toolchain.cmake.
gbalykov
force-pushed
the
fix-tizen-link-args
branch
from
cb38d60 to
5460876
Compare
Re-reading this part, are you saying that it's only tizen x64 that has the problem and this is because we don't set CrossCompileArch (the TargetTriple property ends up being empty and we're not passing If so, does adding |
|
Cross-compiling linux-musl-x64, openbsd-x64 or freebsd-x64 on linux-x64 would infer cross build based on composite "target and host OS + arch combination are not same". Tizen is odd one out because we decided to identify. I don't think what we have in main is flawed, but rather, it's either:
If it's 2, then we shouldn't detect distros. There are over 600 linux distros and we will end up maintaining the database here. Best to figure out why we think tizen needs a different flag. If it's 1. then we should just support |
What I wonder is if E.g. what if we updated this line to also check if a SysRoot was provided: |
|
In our engineering infra (to build runtime), we set In BuildIntegration, we can make it all pinned to Sysroot and update these as well: eng/common/native/LocateNativeCompiler.targets: <SysRoot Condition="'$(CrossBuild)' == 'true'">$(ROOTFS_DIR)</SysRoot>
eng/targetingpacks.targets: <SysRoot Condition="('$(CrossBuild)' == 'true' or '$(BuildArchitecture)' != '$(TargetArchitecture)') and '$(HostOS)' != 'windows'">$(ROOTFS_DIR)</SysRoot>(needs testing) TL;DR, it is not straight-forward CrossBuild=>Sysroot currently, but it can be done. |
Yes, only x64 tizen build has this problem. However, adding Initially I also thought whether cross compilation can just be set based on presence of SysRoot, but decided to not do this since it will affect other platforms. |
4 participants
Without this change linker flags set in src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets in SetupOSSpecificProps do not consider tizen platform specifics, which are set in toolchain.cmake.